Add a dual aggregation presolver for binary variables with a single lock#901
Add a dual aggregation presolver for binary variables with a single lock#901aliceb-nv wants to merge 501 commits intoNVIDIA:release/26.04from
Conversation
This reverts commit 8c5e9f6.
…ed concurrent solution when the B&B is already running. moved number of threads in the probing cache to the setting struct.
|
🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you! If this is an "epic" issue, then please add the "epic" label to this issue. |
2 similar comments
|
🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you! If this is an "epic" issue, then please add the "epic" label to this issue. |
|
🔔 Hi @anandhkb, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you! If this is an "epic" issue, then please add the "epic" label to this issue. |
aliceb-nv
force-pushed
the
setppc-presolve
branch
from
8057774 to
4caf1f1
Compare
|
/ok to test 4caf1f1 |
|
/ok to test b1f7657 |
aliceb-nv
force-pushed
the
setppc-presolve
branch
from
b1f7657 to
a2b498b
Compare
|
/ok to test a2b498b |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cpp/tests/mip/presolve_test.cpp (1)
186-215: Consider adding test for DOWN direction.All current tests cover the UP direction (negative objective coefficient, single up-lock). The algorithm also handles the symmetric DOWN direction (positive objective coefficient, single down-lock). Adding a test case would improve coverage of the symmetric logic path.
Example test structure:
// x has one down-lock in a GEQ row. Prove y=1 => x=1 via activity. // min x // positive objective => prefers decrease // s.t. -3x + 4y >= -1 // GEQ: positive coeff gives down-lock // x, y in {0,1}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cpp/tests/mip/presolve_test.cpp` around lines 186 - 215, Add a symmetric test for the DOWN direction to cover the case where the objective coefficient is positive and a single down-lock exists: create a new TEST (e.g., SingleLockDualAggregation.FavorableStateRejectsDown) that mirrors FavorableStateRejects but uses a positive objective for x and a GEQ row with a negative coeff on x / positive on y (e.g., -3x + 4y >= -1) so the algorithm follows the down-lock path; instantiate the same presolver SingleLockDualAggregation<double>, run it with papilo_harness_t::run, and assert the same expectations (presolve status kUnchanged and zero reductions) to validate the symmetric logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cpp/src/dual_simplex/bounds_strengthening.cpp`:
- Around line 214-221: The code currently reverts small tightenings (using
lb_updated/ub_updated and restoring new_lb=new_ub=old_*) before checking
infeasibility; instead compute tentative new_lb/new_ub from
lower_bounds[k]/upper_bounds[k] without rollback, perform the infeasibility test
new_lb > new_ub + settings.primal_tol first, and only after that apply the
small-change rollback using the 1e3 * settings.primal_tol threshold to decide
whether to keep the tightening for propagation (i.e., restore to old_lb/old_ub
only for propagation decisions, not before the feasibility check). Ensure you
update the places referencing new_lb/new_ub, lb_updated, ub_updated, old_lb,
old_ub, lower_bounds, upper_bounds, and settings.primal_tol accordingly.
---
Nitpick comments:
In `@cpp/tests/mip/presolve_test.cpp`:
- Around line 186-215: Add a symmetric test for the DOWN direction to cover the
case where the objective coefficient is positive and a single down-lock exists:
create a new TEST (e.g., SingleLockDualAggregation.FavorableStateRejectsDown)
that mirrors FavorableStateRejects but uses a positive objective for x and a GEQ
row with a negative coeff on x / positive on y (e.g., -3x + 4y >= -1) so the
algorithm follows the down-lock path; instantiate the same presolver
SingleLockDualAggregation<double>, run it with papilo_harness_t::run, and assert
the same expectations (presolve status kUnchanged and zero reductions) to
validate the symmetric logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5081af3c-47a9-46d4-afdb-c47d3bc5b87f
📒 Files selected for processing (5)
cpp/src/dual_simplex/bounds_strengthening.cppcpp/src/mip_heuristics/presolve/single_lock_dual_aggregation.cppcpp/src/mip_heuristics/presolve/third_party_presolve.cppcpp/src/pdlp/termination_strategy/infeasibility_information.cucpp/tests/mip/presolve_test.cpp
✅ Files skipped from review due to trivial changes (1)
- cpp/src/pdlp/termination_strategy/infeasibility_information.cu
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/src/mip_heuristics/presolve/third_party_presolve.cpp
| if (lb_updated) | ||
| new_lb = std::max(new_lb, lower_bounds[k]); | ||
| else | ||
| new_lb = old_lb; | ||
| if (ub_updated) | ||
| new_ub = std::min(new_ub, upper_bounds[k]); | ||
| else | ||
| new_ub = old_ub; |
There was a problem hiding this comment.
Keep sub-threshold tightenings for the feasibility check.
Line 214 through Line 221 revert any change smaller than 1e3 * settings.primal_tol before Line 223 checks new_lb > new_ub + settings.primal_tol. That can miss a real contradiction when both deductions are small: e.g. the current pass computes new_lb = 6e-4 and new_ub = 4e-4; both deltas fall under the default 1e-3 cutoff, so the code restores the old bounds and returns feasible even though the variable is already infeasible by 2e-4 > primal_tol.
Please defer the rollback until after the infeasibility test, and use the larger threshold only for deciding whether to propagate another iteration.
Possible fix
bool lb_updated = std::abs(new_lb - old_lb) > 1e3 * settings.primal_tol;
bool ub_updated = std::abs(new_ub - old_ub) > 1e3 * settings.primal_tol;
- if (lb_updated)
- new_lb = std::max(new_lb, lower_bounds[k]);
- else
- new_lb = old_lb;
- if (ub_updated)
- new_ub = std::min(new_ub, upper_bounds[k]);
- else
- new_ub = old_ub;
+ if (lb_updated) { new_lb = std::max(new_lb, lower_bounds[k]); }
+ if (ub_updated) { new_ub = std::min(new_ub, upper_bounds[k]); }
if (new_lb > new_ub + settings.primal_tol) {
settings.log.debug(
"Iter:: %d, Infeasible variable after update %d, %e > %e\n", iter, k, new_lb, new_ub);
last_nnz_processed = nnz_processed;
return false;
}
+
+ if (!lb_updated) { new_lb = old_lb; }
+ if (!ub_updated) { new_ub = old_ub; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cpp/src/dual_simplex/bounds_strengthening.cpp` around lines 214 - 221, The
code currently reverts small tightenings (using lb_updated/ub_updated and
restoring new_lb=new_ub=old_*) before checking infeasibility; instead compute
tentative new_lb/new_ub from lower_bounds[k]/upper_bounds[k] without rollback,
perform the infeasibility test new_lb > new_ub + settings.primal_tol first, and
only after that apply the small-change rollback using the 1e3 *
settings.primal_tol threshold to decide whether to keep the tightening for
propagation (i.e., restore to old_lb/old_ub only for propagation decisions, not
before the feasibility check). Ensure you update the places referencing
new_lb/new_ub, lb_updated, ub_updated, old_lb, old_ub, lower_bounds,
upper_bounds, and settings.primal_tol accordingly.
3 participants
This PR adds a dual aggregation pass to Papilo to recognize cases where a binary variable$x$ has only a single lock preventing it from reaching its objective-favorable bound. When found, the pass attempts to substitute $x$ with a master binary variable $y$ present in the locking row.
This new pass attempts to perform a substitution of$x$ with another binary variable y in the row that locks $x$ .
For this, a probing is performed to try to prove that y = 0 => x = 0. We do this by showing x=1 AND y=0 violates the single locking row's activity bounds.
We also prove that y = 1 => x = 1 by showing that the constraint is always satisfied when y = 1, and the objective is not worsened by setting x to 1.
If these two clauses are true, the subsitution
x = ycan be safely performed.We also handle the reverse case of a single down-lock.
This pass runs in O(nnz) time.
With these changes, we solve
neos-787933at the root, thus adding +1 optimal in 10min runs.Description
Issue
Checklist